we don't need support* image tags

So the main goal of this PR to not push them to Docker Hub anymore?

PRs to them are already covered

Partially. We don't require that PRs are rebased on the latest version on the target branch, so there's a small chance that merging an older PR results in an issue on the target branch.

we don't need support* image tags

So the main goal of this PR to not push them to Docker Hub anymore?

👍

PRs to them are already covered

Partially. We don't require that PRs are rebased on the latest version on the target branch, so there's a small chance that merging an older PR results in an issue on the target branch.

The chance that such issue occurs only in the Docker GHA is even smaller.

That sounds more like a workaround for limitations in Icinga/docker-icingadb, which is a project we control, so why should we implement a workaround here?

Though that raises the question whether we want to fix something in Icinga/docker-icingadb or rather just move towards an in-project Dockerfile with a workflow using the standard docker/* actions like we already started doing in Icinga Notifications (https://github.com/Icinga/icinga-notifications/blob/main/Dockerfile, https://github.com/Icinga/icinga-notifications/blob/main/.github/workflows/docker.yml) and Icinga for Kubernetes (https://github.com/Icinga/icinga-kubernetes/blob/main/Containerfile, https://github.com/Icinga/icinga-kubernetes/blob/main/.github/workflows/docker-image.yml).

• For completeness, this is just a follow-up of GHA: Docker: don't run on support/* branches icinga2#10048

To be honest, I don't quite understand your discussion as images are really not needed for support branches and a similar PR has already been merged in Icinga 2. But I welcome moving the Dockerfile here, which still requires environment variable support to be merged, but that is something that is definitely coming.

@Al2Klimov please answer #758 (comment). I'm also wondering about the "we don't need support* image tags" wording. These branches are supposed to have tags for which an image should be built and published. But I think you mean that we tag the image with the name of the support branch? In that case, I would just write "images for support branches". Anyway, please clarify.

@julianbrost any objections to just merging?

@julianbrost any objections to just merging?

Not really. You'd probably have to try very hard to introduce a problem that was only caught by that single job. Still, I find it strange that we have to remove the whole job just to prevent the upload. I'd just find it much nicer if every single commit would run the same set of checks before and after merging, avoiding the whole "is this job strictly necessary in this situation" consideration.

@julianbrost any objections to just merging?

Not really. You'd probably have to try very hard to introduce a problem that was only caught by that single job. Still, I find it strange that we have to remove the whole job just to prevent the upload. I'd just find it much nicer if every single commit would run the same set of checks before and after merging, avoiding the whole "is this job strictly necessary in this situation" consideration.

Sorry to ask again 😆, but I think I have understood the point: You still want to create the image, but just not push it, right? If so, I second that, but I would like to cover this when moving the Dockerfile here.

You still want to create the image, but just not push it, right?

Yes, exactly. For all the other jobs we don't ask the question "do we need the build artifacts for anything afterwards" either. And my intuition says that adding a simple if: to the push step would be preferred, the problem is just that with the current icinga/docker-icingadb project structure, that would be a more complex cross-repo change for something that we plan to phase out. So if the container stuff is moved back into this repo, I'd say there's a good chance the job is run again on the support/* branches, just without pushing the image anywhere.

images are really not needed for support branches and a similar PR has already been merged in Icinga 2.

Exactly my opinion.

I'm also wondering about the "we don't need support* image tags" wording. These branches are supposed to have tags for which an image should be built and published. But I think you mean that we tag the image with the name of the support branch?

Exactly what I meant. This PR is just about for which branches that GHA runs and pushes images named after the respective branch. It has nothing to do with tags.

any objections to just merging?

Not really.

Well, then...

You'd probably have to try very hard to introduce a problem that was only caught by that single job.

If we break it intentionally this way, we deserve it. :-)